Skip to content

Conversation

@josenavas
Copy link
Contributor

I moved the update_category function to the base class. This PR includes the commits in #1044 so review/merge that one first.

I just moved the code around and created the tests for the prep template class.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 57ab913 on josenavas:unify-update-category into * on biocore:master*.

@ElDeveloper
Copy link
Contributor

👍

@josenavas
Copy link
Contributor Author

Now that #1044 has been merged, this should be faster to review!

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.0%) to 79.04% when pulling bb63094 on josenavas:unify-update-category into adff033 on biocore:master.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not do self[k][category] = v and not need to instantiate sample.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're not saving the instantiation, it is happening at the moment that you do self[k]. Anyways, this is left like this because I modify this in my other PR #1054....

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@squirrelo
Copy link
Contributor

One small comment.

@josenavas
Copy link
Contributor Author

Thanks @squirrelo I just answered it, if you think that's ok, it is mergeable!

@squirrelo
Copy link
Contributor

👍

squirrelo added a commit that referenced this pull request Apr 9, 2015
@squirrelo squirrelo merged commit 3ad2264 into qiita-spots:master Apr 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants